Making each test handle its own subscriptions seems problematic.

Also, if we use BiDi in the harness itself, allowing the test to unsubscribe from events could break the harness (we have some ideas to help with this in general, but that requires BiDi changes).

In order to address the concern, we need to add a dependency between testharness and testdriver. I see the following ways:

1. Make testharness aware of the events concept and handle subscriptions there. The events are still implemented in testdriver:

...
promise_test(async (test) => {
    ...
    const log_entry_promise = test.events["log.entryAdded"].once();
    ...
}, "Assert log event is logged", {events: {"log.entryAdded": test_driver.bidi.log.entry_added}});

The test_driver is required to allow testharness managing the subscriptions. The user is supposed to use events via testharness ( test.events...). The specific API can be discussed.
2. Make testdriver aware of testharness:

promise_test(async (test) => {
    ...
    await test_driver.bidi.log.entry_added.subscribe(test);
    ...
}, "Assert log event is logged");

This will not completely address the concern of testharness relying on events.
3. Do not unsubscribe from the events.

I find the option 1 is the preferable. WDYT?

I didn't really mean each test() in a page, but between top-level test files.

The problem is that if the test file is allowed unfettered access to BiDi event subscriptions we can't isolate it from any subscriptions that the harness itself depends on, and doing something like closing the bidi session after each test would be very high overhead. The most obvious example would be if we wanted to use script.message to drive the harness, but also allow scripts themselves to emit custom messages. But it could also happen that, for example, we wanted the harness to be able to subscribe to console.log messages and re-emit them in the harness logs. Allowing the test to unsubscribe from those events would break that. So probably the harness has to manage subscriptions and know which events it subscribed to, and which should be forwarded back to the browser.

Would it be sufficient if we implement automatic clean up of events similar to how BiDi's own test suite does it https://github.com/web-platform-tests/wpt/blob/d7b9fab5017587079985d5ab29781fcf3043533e/webdriver/tests/support/fixtures_bidi.py#L50 ? we could have executewebdriver.py record all subscribe calls coming from the test file and run corresponding unsubscribe calls filtering out events required by the testharness? In that case, unsubscribe calls would not be exposed to testdriver. There will probably be some edge cases around unsubscribing context-specific and global subscriptions at the same time.

Alternative ideas would require changing the spec:

1. support multiple light-weight BiDi sessions that do not require starting/shutting down the browser.
2. returning a subscriptionId in subscribe calls so that only specific subscriptions could be cleaned up.

I believe the API in option 1 partly addresses the concerns you mentioned. It allows implementing event subscription and filtering in the testharness without exposing that logic to the test file, but the specific filtering of let's say script.message intended to be send to the test or to the testharness can be tricky.

So probably the harness has to manage subscriptions and know which events it subscribed to, and which should be forwarded back to the browser.

Do you mean the backend testharness, or the client-side?

FWIW @OrKoN's option 2 is something we'd like to propose for the BiDi spec (although it might have to be opt-in to require a handle to unsubscribe since that's not how things currently work).

The proper long-term solution would be to provide separate sessions for the testharness and for each test files. This would provide proper isolations and tear down.

As an intermediate solution, in the new revision of the example PR I implemented clean up on the wptrunner side as a part of BidiEventsProtocolPart. This approach is used in the WPT BiDi tests (fixtures). This allowed to remove unsubscribe from the testdriver API, but we can always add it, if we have a valid use case.

@jgraham WDYT? If you think this approach makes sense, I will update the RFC accordingly.

I updated the RFC and the example with the new version. The protocol part BidiEventsProtocolPart has a method unsubscribe_all, which is responsible for keeping track of all the subscriptions.

Looking at the PR, I don't like the way this is continuing to use ActionContext.

One of the big advantages of BiDi from the wpt point of view is that it should be possible to communicate with any window, not just the one that currently has WebDriver focus. This proposal seems to end up always switching the WebDriver current browsing context to the one that's getting the BiDi command, which in my opinion rather defeats the point.

Also the use of the ActionContext seems possibly unsound; if you have multiple actions running concurrently I think they can switch the current browsing context out from under each other.

Actually, the ActionContext is not needed in the AsyncCallbackHandler, as there is no need in switching to the required context. Updated the example.

Updated the AsyncCallbackHandler section and example

It seems like for BiDi we should be looking to not have the resume script at all, but instead have the harness emit custom script.message events to communicate information with the harness. That's a bigger change, but I'd like to at least have a sketch of where we're going here, to be sure we aren't backing ourselves into a corner.

Is it s/with the harness/with the testdriver, or I misread the comment?

... but instead have the harness emit custom script.message events to communicate information with the testdriver...

Do you mean in case of BiDi, the testharness should still make a long poll for getting commands from testdriver, but emit message events directly to window, avoiding __wptrunner_process_next_event? I'm not sure if I'm getting the point.

s/with the harness/with the browser/ yes.

In general the current way the testdriver event queue works, with __process_next_event is a workaround for limitations in WebDriver classic. I've been assuming that once we have BiDi we'd be able to move away from that model of polling an event queue in a single window, and instead use the browser event loop directly, and communicate via WebDriver BiDi events rather than blocking script evaluation.

We don't necessarily have to do that right away, but I'd at least like to see what the migration path looks like,

Do you mean in case of BiDi, the testharness should still make a long poll for getting commands from testdriver, but emit message events directly to window, avoiding __wptrunner_process_next_event? I'm not sure if I'm getting the point.

Oh, I got it. You mean the harness can provide a channel for the client-side wptrunner. I guess it is possible and makes sense, but I'd prefer to consider it in a different RFC to reduce execution risks.

If you have some time to prototype that I'd appreciate it. As mentioned one concern in general is that making the wrong first step could make it harder in the long run if we bake in assumptions that don't work for other use cases.

Since the communication between the testharness and the browser is internal and it is not exposed to test authors, it looks like it can be addressed separately. What assumptions from this RFC could prevent us from changing the event implementation in the future?

It sounds like a preload script + a channel for setting up biderectional communication would not require API changes on the testdriver.js side.